Display resource endpoints for Docker Compose deploy#13216
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13216Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13216" |
256820a to
3c202d9
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to display resource endpoints after Docker Compose deployment, addressing issue #12820. The implementation uses pipeline steps to query running Docker Compose containers and reports their accessible endpoints to users.
Key changes:
- Adds endpoint discovery and display functionality using
docker compose ps --format json - Refactors Docker Compose argument building into a reusable method
- Integrates endpoint printing into the deployment pipeline workflow
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Aspire.Hosting.Docker/DockerComposeServiceResource.cs | Converts from primary constructor to explicit constructor; adds PrintEndpointsAsync method to query and display container endpoints after deployment; adds JSON deserialization classes for parsing docker compose output; registers pipeline step for endpoint printing |
| src/Aspire.Hosting.Docker/DockerComposeEnvironmentResource.cs | Extracts docker compose argument building logic into GetDockerComposeArguments helper method; changes GetEnvFilePath to static method; adds pipeline step dependency to ensure endpoint printing runs after docker-compose-up; adds --project-name argument to docker compose commands |
| if (endpointMapping.IsExternal || scheme is "http" or "https") | ||
| { | ||
| var endpoint = $"{scheme}://localhost:{publisher.PublishedPort}"; | ||
| endpoints.Add(endpoint); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for determining which endpoints to display could be clearer. The condition endpointMapping.IsExternal || scheme is "http" or "https" is confusing because:
externalEndpointMappingson line 371 already filters to external endpoints- When
FirstOrDefaultreturns no match (line 410),endpointMappingwill be the default struct value whereIsExternal = false - The fallback to checking
scheme is "http" or "https"suggests the intent is to show all http/https ports even without explicit mapping
Consider adding a comment to clarify this logic, or restructuring to make the intent clearer:
// Show endpoint if: it matches an external endpoint mapping, OR it's an http/https port (published ports are external by default)
var hasExplicitMapping = endpointMapping.Resource is not null;
if (hasExplicitMapping || scheme is "http" or "https")
{
var endpoint = $"{scheme}://localhost:{publisher.PublishedPort}";
endpoints.Add(endpoint);
}|
|
||
| return steps; | ||
| })); | ||
| } |
There was a problem hiding this comment.
Missing blank line before the XML documentation comment. According to the codebase formatting conventions, there should be a blank line between the closing brace of the constructor and the XML documentation comment for the next member.
| } | |
| } |
| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) | ||
| { |
There was a problem hiding this comment.
The PrintEndpointsAsync method is missing XML documentation. According to the Aspire XML documentation guidelines, internal methods should have brief <summary> tags explaining what they do. Consider adding:
/// <summary>
/// Prints the endpoints for the Docker Compose service after deployment.
/// </summary>| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) | |
| { | |
| /// <summary> | |
| /// Prints the endpoints for the Docker Compose service after deployment. | |
| /// </summary> | |
| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) |
| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) | ||
| { | ||
| var outputPath = PublishingContextUtils.GetEnvironmentOutputPath(context, environment); | ||
| var dockerComposeFilePath = Path.Combine(outputPath, "docker-compose.yaml"); | ||
|
|
||
| if (!File.Exists(dockerComposeFilePath)) | ||
| { | ||
| context.Logger.LogWarning("Docker Compose file not found at {Path}", dockerComposeFilePath); | ||
| return; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| // Use docker compose ps to get the running containers and their port mappings | ||
| var arguments = DockerComposeEnvironmentResource.GetDockerComposeArguments(context, environment); | ||
| arguments += " ps --format json"; | ||
|
|
||
| var outputLines = new List<string>(); | ||
|
|
||
| var spec = new ProcessSpec("docker") | ||
| { | ||
| Arguments = arguments, | ||
| WorkingDirectory = outputPath, | ||
| ThrowOnNonZeroReturnCode = false, | ||
| InheritEnv = true, | ||
| OnOutputData = output => | ||
| { | ||
| if (!string.IsNullOrWhiteSpace(output)) | ||
| { | ||
| outputLines.Add(output); | ||
| } | ||
| }, | ||
| OnErrorData = error => | ||
| { | ||
| if (!string.IsNullOrWhiteSpace(error)) | ||
| { | ||
| context.Logger.LogDebug("docker compose ps (stderr): {Error}", error); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| var (pendingProcessResult, processDisposable) = ProcessUtil.Run(spec); | ||
|
|
||
| await using (processDisposable) | ||
| { | ||
| var processResult = await pendingProcessResult | ||
| .WaitAsync(context.CancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| if (processResult.ExitCode != 0) | ||
| { | ||
| context.Logger.LogWarning("Failed to query Docker Compose services for {ResourceName}. Exit code: {ExitCode}", TargetResource.Name, processResult.ExitCode); | ||
| return; | ||
| } | ||
|
|
||
| // Parse the JSON output to find port mappings for this service | ||
| var serviceName = TargetResource.Name.ToLowerInvariant(); | ||
| var endpoints = new HashSet<string>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| // Get all external endpoint mappings for this resource | ||
| var externalEndpointMappings = EndpointMappings.Values.Where(m => m.IsExternal).ToList(); | ||
|
|
||
| // If there are no external endpoints configured, we're done | ||
| if (externalEndpointMappings.Count == 0) | ||
| { | ||
| context.ReportingStep.Log(LogLevel.Information, $"Successfully deployed **{TargetResource.Name}** to Docker Compose environment **{environment.Name}**. No public endpoints were configured.", enableMarkdown: true); | ||
| return; | ||
| } | ||
|
|
||
| foreach (var line in outputLines) | ||
| { | ||
| try | ||
| { | ||
| var serviceInfo = JsonSerializer.Deserialize(line, DockerComposeJsonContext.Default.DockerComposeServiceInfo); | ||
|
|
||
| if (serviceInfo is null || | ||
| !string.Equals(serviceInfo.Service, serviceName, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| if (serviceInfo.Publishers is not { Count: > 0 }) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| foreach (var publisher in serviceInfo.Publishers) | ||
| { | ||
| // Skip ports that aren't actually published (port 0 or null means not exposed) | ||
| if (publisher.PublishedPort is not > 0) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| // Try to find a matching external endpoint to get the scheme | ||
| // Match by internal port (numeric) or by exposed port | ||
| // InternalPort may be a placeholder like ${API_PORT} for projects, so also check ExposedPort | ||
| var targetPortStr = publisher.TargetPort?.ToString(CultureInfo.InvariantCulture); | ||
| var endpointMapping = externalEndpointMappings | ||
| .FirstOrDefault(m => m.InternalPort == targetPortStr || m.ExposedPort == publisher.TargetPort); | ||
|
|
||
| // If we found a matching endpoint, use its scheme; otherwise default to http for external ports | ||
| var scheme = endpointMapping.Scheme ?? "http"; | ||
|
|
||
| // Only add if we found a matching external endpoint OR if scheme is http/https | ||
| // (published ports are external by definition in docker compose) | ||
| if (endpointMapping.IsExternal || scheme is "http" or "https") | ||
| { | ||
| var endpoint = $"{scheme}://localhost:{publisher.PublishedPort}"; | ||
| endpoints.Add(endpoint); | ||
| } | ||
| } | ||
| } | ||
| catch (JsonException ex) | ||
| { | ||
| context.Logger.LogDebug(ex, "Failed to parse docker compose ps output line: {Line}", line); | ||
| } | ||
| } | ||
|
|
||
| // Display the endpoints | ||
| if (endpoints.Count > 0) | ||
| { | ||
| var endpointList = string.Join(", ", endpoints.Select(e => $"[{e}]({e})")); | ||
| context.ReportingStep.Log(LogLevel.Information, $"Successfully deployed **{TargetResource.Name}** to {endpointList}", enableMarkdown: true); | ||
| } | ||
| else | ||
| { | ||
| context.ReportingStep.Log(LogLevel.Information, $"Successfully deployed **{TargetResource.Name}** to Docker Compose environment **{environment.Name}**. No public endpoints were configured.", enableMarkdown: true); | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| context.Logger.LogWarning(ex, "Failed to retrieve endpoints for {ResourceName}", TargetResource.Name); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> |
There was a problem hiding this comment.
The new endpoint printing functionality introduced in the PrintEndpointsAsync method and the pipeline step configuration lacks test coverage. Consider adding tests to verify:
- Endpoint discovery and display when containers are running
- Behavior when no external endpoints are configured
- Handling of multiple endpoints with different schemes
- Error handling when Docker Compose commands fail
The test file tests/Aspire.Hosting.Docker.Tests/DockerComposePublisherTests.cs has comprehensive test coverage for other Docker Compose functionality and would be an appropriate location for these tests.
captainsafia
left a comment
There was a problem hiding this comment.
@copilot Address this feedback.
| var printResourceSummary = new PipelineStep | ||
| { | ||
| Name = $"print-{resource.Name}-summary", | ||
| Action = async ctx => await PrintEndpointsAsync(ctx, composeEnvironmentResource).ConfigureAwait(false), |
There was a problem hiding this comment.
| Action = async ctx => await PrintEndpointsAsync(ctx, composeEnvironmentResource).ConfigureAwait(false), | |
| Action = async ctx => await PrintEndpointsAsync(ctx, _composeEnvironmentResource).ConfigureAwait(false), |
|
|
||
| var printResourceSummary = new PipelineStep | ||
| { | ||
| Name = $"print-{resource.Name}-summary", |
There was a problem hiding this comment.
| Name = $"print-{resource.Name}-summary", | |
| Name = $"print-{_targetResource.Name}-summary", |
| _composeEnvironmentResource = composeEnvironmentResource; | ||
|
|
||
| // Add pipeline step annotation to display endpoints after deployment | ||
| Annotations.Add(new PipelineStepAnnotation((factoryContext) => |
There was a problem hiding this comment.
| Annotations.Add(new PipelineStepAnnotation((factoryContext) => | |
| Annotations.Add(new PipelineStepAnnotation(_ => |
|
|
||
| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) | ||
| { | ||
| var outputPath = PublishingContextUtils.GetEnvironmentOutputPath(context, environment); |
There was a problem hiding this comment.
Remove this declaration and check for dockerComposeFilePath since we do it in DockerComposeEnvironmentResource.GetDockerComposeArguments now.
|
@captainsafia I've opened a new pull request, #13217, to work on those changes. Once the pull request is ready, I'll request review from you. |
…3217) * Initial plan * Address review feedback on DockerComposeServiceResource Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
| try | ||
| { | ||
| // Use docker compose ps to get the running containers and their port mappings | ||
| var arguments = DockerComposeEnvironmentResource.GetDockerComposeArguments(context, environment); |
There was a problem hiding this comment.
@copilot add a comment that shows an example of the expected format being parsed here from docker compose.
There was a problem hiding this comment.
@copilot add a comment that shows an example of the expected format being parsed here from docker compose.
|
@davidfowl I've opened a new pull request, #13220, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@davidfowl I've opened a new pull request, #13222, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
It's not printing the dashboard url. #:package Aspire.Hosting.Docker@13.1.0-pr.13216.g40a62e5e
#:sdk Aspire.AppHost.Sdk@13.1.0-pr.13216.g40a62e5e
var builder = DistributedApplication.CreateBuilder(args);
builder.AddDockerComposeEnvironment("env")
.WithDashboard(db => db.WithHostPort(9090));
#pragma warning disable ASPIRECSHARPAPPS001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
builder.AddCSharpApp("api3", "./api.cs")
.WithHttpEndpoint(targetPort: 5000)
.WithExternalHttpEndpoints();
builder.Build().Run(); |
| } | ||
| } | ||
|
|
||
| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) |
There was a problem hiding this comment.
Doesn't this need to handle potential slow startup of services? What guaranteees that docker compose ps --format json will have the information by the time you call it?
Yep, I haven't done anything around that in this PR. Although I wonder if printing the dashboard URL might be a better strategy since it launches deterministically and can be used at the gateway to discover service URIs. |
|
If you have it enabled, we should show it. I was playing with this in my ssh docker deploy and we should make sure that it works well if containers don't start up immediately. |
|
Will follow up with some improvements. This makes the experience of using compose a bit better even if it is not complete. |
Closes #12820.